-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[clr-interp] Convert call stub logging to LOG2 facility #123273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Added LF2_INTERPRETER to log facility enum in log.h - Converted all #if LOG_COMPUTE_CALL_STUB blocks to use LOG2 macro with LF2_INTERPRETER and LL_INFO10000 - Removed #define LOG_COMPUTE_CALL_STUB 0 definition - Updated all printf statements to use LOG2 instead Co-authored-by: davidwrighton <10779849+davidwrighton@users.noreply.github.com>
Co-authored-by: davidwrighton <10779849+davidwrighton@users.noreply.github.com>
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR converts interpreter call stub logging from compile-time preprocessor directives to runtime-controllable LOG2 facility, making debugging more convenient without requiring recompilation.
Changes:
- Added
LF2_INTERPRETERto the LogFacility2 enum for runtime-controlled interpreter logging - Converted 26 compile-time
#if LOG_COMPUTE_CALL_STUBlogging blocks to runtimeLOG2((LF2_INTERPRETER, LL_INFO10000, ...))calls - Removed the
LOG_COMPUTE_CALL_STUBpreprocessor define and updated associated macros
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/coreclr/inc/log.h | Added LF2_INTERPRETER = 0x00000002 to LogFacility2 enum for interpreter logging |
| src/coreclr/vm/callstubgenerator.cpp | Removed LOG_COMPUTE_CALL_STUB define and converted all 26 logging statements from compile-time #if blocks to runtime LOG2 calls; updated INVOKE_FUNCTION_PTR and RETURN_TYPE_HANDLER macros |
## Description
Converts interpreter call stub logging from compile-time `#if
LOG_COMPUTE_CALL_STUB` blocks to runtime-controllable LOG2 facility.
These logging statements have proven valuable for debugging but required
recompilation to enable.
## Changes
- Added `LF2_INTERPRETER = 0x00000002` to LogFacility2 enum
- Converted 26 logging instances to `LOG2((LF2_INTERPRETER,
LL_INFO10000, ...))`
- Removed `#define LOG_COMPUTE_CALL_STUB 0`
- Updated `INVOKE_FUNCTION_PTR` and `RETURN_TYPE_HANDLER` macros
### Before
```cpp
#define LOG_COMPUTE_CALL_STUB 0
PCODE GetGPRegRangeRoutine(int r1, int r2)
{
#if LOG_COMPUTE_CALL_STUB
printf("GetGPRegRangeRoutine %d %d\n", r1, r2);
#endif
// ...
}
```
### After
```cpp
PCODE GetGPRegRangeRoutine(int r1, int r2)
{
LOG2((LF2_INTERPRETER, LL_INFO10000, "GetGPRegRangeRoutine %d %d\n", r1, r2));
// ...
}
```
Logging now enabled via: `DOTNET_LogFacility2=0x2 DOTNET_LogLevel=10000`
<!-- START COPILOT ORIGINAL PROMPT -->
<details>
<summary>Original prompt</summary>
>
> ----
>
> *This section details on the original issue you should resolve*
>
> <issue_title>[clr-interp] Convert logging in call stubs to use logging
facility</issue_title>
> <issue_description>I think we should convert it to the LOG mechanism.
When I wrote these logging statements, I had thought they would likely
be very temporary, but I've used them 4-5 times since then, so they have
some ongoing value. However, I'd like to see that work done in a
separate PR. We should convert them to using LOG statements probably
with a new LF enum value. Probably what should be done is add a new log
facility enum in log.h, call it LF2_INTERPRETER. And convert these log
statements to use the LOG2 macro, with info level LL_INFO10000 instead
of wrapping them in #if LOG_COMPUTE_CALL_STUB blocks.
>
> _Originally posted by @davidwrighton in
dotnet#123142 (comment)
> </issue_description>
>
> ## Comments on the Issue (you are @copilot in this section)
>
> <comments>
> </comments>
>
</details>
<!-- START COPILOT CODING AGENT SUFFIX -->
- Fixes dotnet#123272
<!-- START COPILOT CODING AGENT TIPS -->
---
✨ Let Copilot coding agent [set things up for
you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot)
— coding agent works faster and does higher quality work when set up for
your repo.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: davidwrighton <10779849+davidwrighton@users.noreply.github.com>
Co-authored-by: David Wrighton <davidwr@microsoft.com>
Description
Converts interpreter call stub logging from compile-time
#if LOG_COMPUTE_CALL_STUBblocks to runtime-controllable LOG2 facility. These logging statements have proven valuable for debugging but required recompilation to enable.Changes
LF2_INTERPRETER = 0x00000002to LogFacility2 enumLOG2((LF2_INTERPRETER, LL_INFO10000, ...))#define LOG_COMPUTE_CALL_STUB 0INVOKE_FUNCTION_PTRandRETURN_TYPE_HANDLERmacrosBefore
After
Logging now enabled via:
DOTNET_LogFacility2=0x2 DOTNET_LogLevel=10000Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.